-
-
Notifications
You must be signed in to change notification settings - Fork 358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable platform quests on relations #5183
Conversation
Have you tested this pull request? |
I did not test the changes so far - this was more intendet to outline which quests are (probably) affected and the changes that I think solve the issue. Should have made that more clear, though. I will change this to a draft. |
I now have checked that the bench, bin, is lit, shelter and name quests are now also displayed for relation platforms and are answerable. The quests also (still) come up if the platform is a node or way. (Note: I did not change anything on the bus stop ref quest, because I am not sure whether that one intentionally just searches for nodes.) I'm not sure what else could be affected by the change, so if anyone with more insight into SC internals can point me to other functionality that requires testing, I'd be very grateful. |
That seems to be about it. StreetComplete Quests are self-contained, so no other quest should behave differently except the quests you changed. As you tested them they work OK for both old and new behaviour, it looks good to me. Feel free to mark it as
Good question. I recall some issues with problems when some transport information is mapped both as node and way (see e.g. #5132 and linked issues), so I'm not sure if there is a reason for that. But that one can be always be added as separate PR if that turns out to be safe to do (and if not safe, another PR documenting in the code why it only looks for nodes would be welcome.) |
Hmm, I think nothing speaks against this... The bus stop quests especially in regard to railway platforms have been subject of various discussions in the past, but I guess since railway platforms are now also included in the quest, it makes sense to also look for relations (i.e. railway platforms with a hole to accommodate stairs). Looking at git blame, other than me, @FloEdelmann has some lines of code there, so I am just going to ping him if he has any concerns. |
I don't think anything speaks against this :) |
This commit also considers
public_transport=platform
s that are relations (e.g. multipolygons). See #5182 for more details. Fixes #5182.